Skip to content

Fix cloudpathlib error when package not available#62

Merged
kaitj merged 2 commits into
childmindresearch:mainfrom
kaitj:fix/cloudpaths
Sep 10, 2025
Merged

Fix cloudpathlib error when package not available#62
kaitj merged 2 commits into
childmindresearch:mainfrom
kaitj:fix/cloudpaths

Conversation

@kaitj
Copy link
Copy Markdown
Contributor

@kaitj kaitj commented Sep 9, 2025

Resolves #61.

Uses CloudPath as defined in _pathlib.py. Also checks for cloudpathlib availability in first condition (potential definition of CloudPath if library isn't defined).

Additionally adds a test run to the CI without cloudpathlib installed. Note, only the run with cloudpathlib installed generates a coverage report (there is overlap here anyways, aside from the 2 additional tests for cloud datasets). The additional test is more to catch any issues that may crop up without the additional package.

- Use _pathlib.CloudPath instead of cloudpathlib.CloudPath. The import here already has a bunch of logic baked in to check and fallback the pathtype if cloudpathlib isn't available.
- For the conditional logic, make use of the cloudpathlib_is_available function to step into check
- TODO: Add tests to make sure cloudpathlib functionality is working as expected
- Mark additional s3 test with to skip
@kaitj kaitj requested a review from nx10 September 9, 2025 14:59
@kaitj kaitj self-assigned this Sep 9, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 9, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.14%. Comparing base (f3d111c) to head (fdf5552).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #62      +/-   ##
==========================================
- Coverage   95.15%   95.14%   -0.01%     
==========================================
  Files           7        7              
  Lines         495      494       -1     
==========================================
- Hits          471      470       -1     
  Misses         24       24              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kaitj kaitj changed the title Fix/cloudpaths Fix cloudpathlib error when package not available Sep 9, 2025
Copy link
Copy Markdown
Collaborator

@nx10 nx10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are ways to combine test reports for codecov etc, but I think it's fine.

@kaitj
Copy link
Copy Markdown
Contributor Author

kaitj commented Sep 10, 2025

There are ways to combine test reports for codecov etc, but I think it's fine.

Yeah, when I was first looking at this, I was thinking that the overlap probably covers it all, but on second thought, it probably doesn't catch some of the branching so would be good to combine test reports. I'll leave that for a separate PR. Going to merge this one in to patch import issue.

@kaitj kaitj merged commit 32ed4a5 into childmindresearch:main Sep 10, 2025
6 checks passed
@kaitj kaitj deleted the fix/cloudpaths branch September 10, 2025 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing cloudpathlib dependency causes ModuleNotFoundError on import

2 participants